NodeResourceTopology updates to match latest kubelet features #920
+376
−141
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
do-not-merge/work-in-progress
✅ Deploy Preview for kubernetes-sigs-scheduler-plugins canceled.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ffromani The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
|
reviewers kindly ignore this PR until I remove the WIP |
ffromani
changed the title
ffromani
force-pushed
the
nrt-update-202508
branch
2 times, most recently
from
5351f4f to
791323d
Compare
add utilities to recognize sidecar containers, and log them correctly. Signed-off-by: Francesco Romani <[email protected]>
ffromani
force-pushed
the
nrt-update-202508
branch
from
791323d to
a06d4c2
Compare
ffromani
commented
Aug 4, 2025
| Policy: "single-numa-node", | ||
| Scope: "container", | ||
| Policy: "single-numa-node", | ||
| MaxNUMANodes: 9, // grachopper |
There was a problem hiding this comment.
typo, will fix in the next upload
add a helper struct to group together all the context we need to carry in the internal implementation functions. The goals are to streamline the existing code, to reduce redundant call and to make room for more arguments to be passed through, like the topology manager configuration. Signed-off-by: Francesco Romani <[email protected]>
similarly to what we did for filter, add a helper struct to group together all the context we need to carry in the internal implementation functions. The goals are to streamline the existing code, to reduce redundant call and to make room for more arguments to be passed through, like the topology manager configuration. Signed-off-by: Francesco Romani <[email protected]>
ffromani
force-pushed
the
nrt-update-202508
branch
from
a06d4c2 to
70a88d1
Compare
k8s-ci-robot
added
size/XL
Add more information, only at high verbosity levels, about the filter behavior; report the precise rejection reason, both in logs and in the user-visible record. The user-visible record (e.g. in pod events) reported in framework.Status must be concise, so we limit to a single extra short word, being it the resource name. Signed-off-by: Francesco Romani <[email protected]>
ffromani
force-pushed
the
nrt-update-202508
branch
from
b6c4579 to
b0badfe
Compare
kubernetes 1.31 gained support for topology manager options, and most notably to bump the limit of max NUMA nodes, which is no longer hardcoded to 8. Thus, we need to adapt the scheduler code. Likewise other key topology manager parameters, we trust NRT objects to carry the values we need. In this case, to mitigate DoS risks, we set an artificial "high enough" (and hopefully good enough for a while) limit. Once obtained the vlaue, we pass it through to all the code places on which the previous hardcoded value was used. Signed-off-by: Francesco Romani <[email protected]>
There is no de-duplication of key/value pairs by default in go-logr or in klog, so we remove data which is not obviously needed and which is likely to be provided, or already provided in some other form (e.g. logger name) from the framework. Signed-off-by: Francesco Romani <[email protected]>
we can use constants added in the helper package and already available in the code we require. No intended changes in behavior. Signed-off-by: Francesco Romani <[email protected]>
ffromani
force-pushed
the
nrt-update-202508
branch
from
b0badfe to
0d3207a
Compare
ffromani
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/hold the PR is reviewable, I'm fine tuning the logs and checking for gaps (hence the hold). I can post a followup tuning later, but since this work is not especially urgent, I'd hold it for a little while and do all in one go. |
k8s-ci-robot
added
the
do-not-merge/hold
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
implement support for relevant topology manager functionalities added since roughly 1.27.
Topology manager gained options similar to cpumanager. We begin the support implementing the scheduler equivalent of MaxAllowedNUMANodes.
detect sidecar containers
improve the logging again
Which issue(s) this PR fixes:
Fixes NA
Special notes for your reviewer:
Likewise the other topology manager settings, the value is node-local so it is better represented as NodeResourceTopology object property rather than a global configuration value.
Does this PR introduce a user-facing change?